feat: add auto_monitoring_config in GKE managed_prometheus #2420
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
tatchiuleung
force-pushed
the
add-auto-monitoring-config
branch
3 times, most recently
from
5151105 to
dafb16b
Compare
tatchiuleung
force-pushed
the
add-auto-monitoring-config
branch
from
dafb16b to
ba27a17
Compare
|
/gcbrun |
|
/gcbrun |
|
/gcbrun |
|
Can we get a review on this? Would be nice to get this added in. |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for auto_monitoring_config within the managed_prometheus block for GKE clusters. A new variable, monitoring_auto_monitoring_config_scope, is introduced to control this feature. The changes are consistently applied across all relevant modules, templates, and documentation files. The implementation is correct and follows the established patterns in the repository. I have one minor suggestion to improve the validation error message for the new variable to make it more descriptive for users.
| ], | ||
| var.monitoring_auto_monitoring_config_scope | ||
| ) | ||
| error_message = "'monitoring_auto_monitoring_config_scope' value is invalid" |
There was a problem hiding this comment.
The validation error message is a bit generic. It would be more helpful to the user if it listed the allowed values.
error_message = "'monitoring_auto_monitoring_config_scope' must be one of 'ALL' or 'NONE'."
| ], | ||
| var.monitoring_auto_monitoring_config_scope | ||
| ) | ||
| error_message = "'monitoring_auto_monitoring_config_scope' value is invalid" |
There was a problem hiding this comment.
The validation error message is a bit generic. It would be more helpful to the user if it listed the allowed values.
error_message = "'monitoring_auto_monitoring_config_scope' must be one of 'ALL' or 'NONE'."
| ], | ||
| var.monitoring_auto_monitoring_config_scope | ||
| ) | ||
| error_message = "'monitoring_auto_monitoring_config_scope' value is invalid" |
There was a problem hiding this comment.
The validation error message is a bit generic. It would be more helpful to the user if it listed the allowed values.
error_message = "'monitoring_auto_monitoring_config_scope' must be one of 'ALL' or 'NONE'."
| ], | ||
| var.monitoring_auto_monitoring_config_scope | ||
| ) | ||
| error_message = "'monitoring_auto_monitoring_config_scope' value is invalid" |
There was a problem hiding this comment.
The validation error message is a bit generic. It would be more helpful to the user if it listed the allowed values.
error_message = "'monitoring_auto_monitoring_config_scope' must be one of 'ALL' or 'NONE'."
| ], | ||
| var.monitoring_auto_monitoring_config_scope | ||
| ) | ||
| error_message = "'monitoring_auto_monitoring_config_scope' value is invalid" |
There was a problem hiding this comment.
The validation error message is a bit generic. It would be more helpful to the user if it listed the allowed values.
error_message = "'monitoring_auto_monitoring_config_scope' must be one of 'ALL' or 'NONE'."
| ], | ||
| var.monitoring_auto_monitoring_config_scope | ||
| ) | ||
| error_message = "'monitoring_auto_monitoring_config_scope' value is invalid" |
There was a problem hiding this comment.
The validation error message is a bit generic. It would be more helpful to the user if it listed the allowed values.
error_message = "'monitoring_auto_monitoring_config_scope' must be one of 'ALL' or 'NONE'."
| ], | ||
| var.monitoring_auto_monitoring_config_scope | ||
| ) | ||
| error_message = "'monitoring_auto_monitoring_config_scope' value is invalid" |
There was a problem hiding this comment.
The validation error message is a bit generic. It would be more helpful to the user if it listed the allowed values.
error_message = "'monitoring_auto_monitoring_config_scope' must be one of 'ALL' or 'NONE'."
| ], | ||
| var.monitoring_auto_monitoring_config_scope | ||
| ) | ||
| error_message = "'monitoring_auto_monitoring_config_scope' value is invalid" |
There was a problem hiding this comment.
The validation error message is a bit generic. It would be more helpful to the user if it listed the allowed values.
error_message = "'monitoring_auto_monitoring_config_scope' must be one of 'ALL' or 'NONE'."
|
/gcbrun |
3 participants
Issue: #2409